Adding support for egress and ingress settings#243
Adding support for egress and ingress settings#243troykelly wants to merge 1 commit intoserverless:masterfrom
Conversation
medikoo
left a comment
medikoo
left a comment
There was a problem hiding this comment.
@troykelly Great thanks for that improvement. Please see my comments, also please ensure that CI passes without errors
| } | ||
|
|
||
| if (funcObject.egress) { | ||
| _.assign(funcTemplate.properties, { |
There was a problem hiding this comment.
Just following existing established formatting.
I will come back to these change requests in a few weeks, we will just have to work with our fork for now.
|
|
||
| if (funcObject.egress) { | ||
| _.assign(funcTemplate.properties, { | ||
| vpcConnectorEgressSettings: _.get(funcObject, 'egress') || _.get(this, 'serverless.service.provider.egress'), |
There was a problem hiding this comment.
_.get(funcObject, 'egress') is guaranteed to be truthy at this logic point
| if (funcObject.ingress) { | ||
| _.assign(funcTemplate.properties, { | ||
| ingressSettings: _.get(funcObject, 'ingress') || _.get(this, 'serverless.service.provider.ingress'), |
| * @param {*} functionName | ||
| */ | ||
| const validateVpcEgressProperty = (funcObject, functionName) => { | ||
| if (funcObject.egress && typeof funcObject.egress === 'string') { |
There was a problem hiding this comment.
Let's do just typeof check (other is superfluous)
| */ | ||
| const validateVpcEgressProperty = (funcObject, functionName) => { | ||
| if (funcObject.egress && typeof funcObject.egress === 'string') { | ||
| const validTypes = ['VPC_CONNECTOR_EGRESS_SETTINGS_UNSPECIFIED', 'PRIVATE_RANGES_ONLY', 'ALL_TRAFFIC']; |
There was a problem hiding this comment.
Let's define it as Set instance and outside of function body
| if (funcObject.ingress && typeof funcObject.ingress === 'string') { | ||
| const validTypes = ['INGRESS_SETTINGS_UNSPECIFIED', 'ALLOW_ALL', 'ALLOW_INTERNAL_ONLY', 'ALLOW_INTERNAL_AND_GCLB']; |
|
Hello, when will this be merged? I need this feature |
@ivanguimam can you help us in finalizing this PR? |
How can I make these corrections being that PR belongs to someone else? |
You can fork the repository, do updates, and propose another PR with updates |
I'll try to do that by the weekend |
@ivanguimam that'll be great. Thank you! |
|
@ivanguimam awesome let us know! |
4 participants
Resolves #242